fix: consume part of StreamingResponseIterator to support failure while under a retry context#10206
Conversation
googlebot
added
the
cla: yes
crwilcox
added
the
do not merge
| try: | ||
| self._first_result = six.next(self._wrapped) | ||
| self._stored_first_result = True | ||
| except StopIteration: |
There was a problem hiding this comment.
Will this work for the WatchStream? For Watch, the client has to send the first request. We will not receive a response until after this request has been processed.
There was a problem hiding this comment.
Watch uses ResumableBidiRpc and has its own way of managing recovery. This will have an effect on things like query though.
tseaver
left a comment
tseaver
left a comment
There was a problem hiding this comment.
I would use only a single instance variable here, and arrange to set / clear it atomically.
|
NOTE: 3 tests look to need refactoring, also should likely improve coverage given this new behavior. |
| # to retrieve the first result, in order to fail, in order to trigger a retry. | ||
| try: | ||
| self._stored_first_result = six.next(self._wrapped) | ||
| except TypeError: |
There was a problem hiding this comment.
@tseaver Under test we have a non-iterable. While it is odd that you would call this without it does seem possible? So added a guard here
crwilcox
changed the title
crwilcox
removed
the
do not merge
…le under a retry context (#10206)
6 participants
In the Node.js SDK, there's code that explicitly waits until at least one result has been observed and retries the underlying stream until then. This doesn't handle the case where the connection is interrupted mid-stream but has been good enough to resolve this class of issues. https://github.com/googleapis/nodejs-firestore/blob/master/dev/src/index.ts#L1127
The Firestore Python SDK needs equivalent code to make it resilient to this class of failure.
This PR causes init of the streaming response iterator to consume the first result immediately which will cause the error to occur while under the retry context. Once the iterator is created it is returned and future access is no longer in a retry context.